files: track relabeling using set instead of slice#1324
Conversation
jlebon
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
I'm not sure if this is worth it. setfiles will handle duplicate entries just fine and in general we shouldn't be generating that many duplicates anyway.
If we really want to make sure that all paths are unique before passing to setfiles, maybe a cleaner approach would be to switch from a slice to a set (really, a map[string]struct{}).
|
Yes I understand, so in #1303 I used to code differents things to prevent sending to relabel too many file/path |
|
Yes, OK, read some of the discussions in #1303 now. That PR is introducing code which will make it easier to have multiple requests for relabeling the same dirs unless the code is made more complex to handle that. So I'm cool with embedding handling for this into the relabeling bits directly. For that, let's do
instead? |
|
That said, I'd consider this orthogonal to #1303. You can go ahead and simplify the code there to just call |
|
Yes, we don't speak about a ton of files, you're right :) I just dive in the
I agree for using |
jlebon
left a comment
There was a problem hiding this comment.
Looks close! Can you also squash all your commits to a single one?
internal/exec/stages/files/files.go
Outdated
| @@ -132,7 +132,7 @@ func (s *stage) checkRelabeling() error { | |||
|
|
|||
| // initialize to non-nil (whereas a nil slice means not to append, even | |||
There was a problem hiding this comment.
Outdated comment. I think the point it makes is still valid for maps, so you can just s/slice/map/.
internal/exec/stages/files/files.go
Outdated
| // labeling for files created by processes we call out to, like `useradd`. | ||
|
|
||
| return s.RelabelFiles(s.toRelabel) | ||
| keys := []string{} |
There was a problem hiding this comment.
Minor: we can use make here instead to preset the capacity since we know how many things we'll add.
dd1bc49 to
ebaa437
Compare
|
Sorry, one final thing. Let's tweak the commit message to something like: ? |
That way we avoid passing the same path to `setfiles` multiple times.
|
Nice work @Nemric! |
This PR handle relabelling by preventing multiple relabels ...
The goal is to avoid managing it in the code allowing contributors to call relabelling func without having to check if the path is already included in the "torelabel" slice